[Issue 843] Moved vertex type map to vertices; split printParameters by model#936
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors per-vertex type ownership from Layout into AllVertices, and adjusts recorder/history registration and layout parameter printing to be model-specific (Neuro vs NG911). The changes touch initialization, connection growth (GPU), recorders, and NG911 connection editing.
Changes:
- Moved
vertexTypeMap_fromLayouttoAllVertices(including recorder registration + cereal serialization). - Updated Neuro/NG911 call sites (vertices, connections, inputs, recorders, GPU code) to read/write types via
layout.getVertices().vertexTypeMap_. - Added model-specific cached index lists in
LayoutNeuro/Layout911and reworkedLayoutNeuro::printParameters()accordingly.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Simulator/Vertices/NG911/All911Vertices.cpp | Updates NG911 vertex logic to use vertex types via Layout::getVertices(); ensures history registration chains to base. |
| Simulator/Vertices/Neuro/AllSpikingNeurons.cpp | Calls base AllVertices::registerHistoryVariables() so vertex types are recorded. |
| Simulator/Vertices/Neuro/AllIZHNeurons.cpp | Switches excitatory/inhibitory checks to the new vertex type storage. |
| Simulator/Vertices/Neuro/AllIFNeurons.cpp | Switches neuron-type dispatch + error logging to the new vertex type storage. |
| Simulator/Vertices/AllVertices.h | Adds vertexTypeMap_ to vertices; changes registerHistoryVariables() to a virtual base implementation; updates serialization. |
| Simulator/Vertices/AllVertices.cpp | Initializes and registers vertexTypeMap_ with the recorder. |
| Simulator/Utils/Inputs/SInputPoisson.cpp | Uses the moved vertex type map when selecting edge types. |
| Simulator/Recorders/NG911/Xml911Recorder.cpp | Reads vertex types from layout.getVertices() during save. |
| Simulator/Layouts/NG911/Layout911.h | Adds NG911 role index lists/count owned by the NG911 layout. |
| Simulator/Layouts/NG911/Layout911.cpp | Populates vertex types via vertices; adds role lists; updates edgType to use new storage. |
| Simulator/Layouts/Neuro/LayoutNeuro.h | Adds cached neuron index lists and serializes them. |
| Simulator/Layouts/Neuro/LayoutNeuro.cpp | Prints model-specific layout parameters using cached lists; updates type map generation and starter tracking. |
| Simulator/Layouts/Layout.h | Makes printParameters() pure-virtual (model-specific only); removes layout-owned type map serialization. |
| Simulator/Layouts/Layout.cpp | Removes layout history registration for vertex types; initializes vertex types via vertices. |
| Simulator/Connections/NG911/Connections911.cpp | Updates NG911 connection-editing logic to mutate vertex types via vertices. |
| Simulator/Connections/Neuro/ConnGrowth_d.cpp | Copies neuron type map to device using the new vertices-owned storage. |
| .gitignore | Ignores additional Debug log filename patterns. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
Simulator/Layouts/NG911/Layout911.cpp:163
- Same issue as above for the responder->CALR case: the condition checks FIRE/EMS against destVertex instead of srcVertex. As written, FIRE/EMS responders won’t be classified as responder sources here. Adjust the predicate to test vtypes[srcVertex] for LAW/FIRE/EMS and vtypes[destVertex] for CALR.
else if ((vtypes[srcVertex] == vertexType::LAW || vtypes[destVertex] == vertexType::FIRE
|| vtypes[destVertex] == vertexType::EMS)
&& vtypes[destVertex] == vertexType::CALR)
return edgeType::RC;
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| /// Prints out all parameters to logging file. Registered to OperationManager as Operation::printParameters | ||
| void Layout::printParameters() const | ||
| { | ||
| GraphManager<NeuralVertexProperties>::VertexIterator vi, vi_end; | ||
| GraphManager<NeuralVertexProperties> &gm = GraphManager<NeuralVertexProperties>::getInstance(); | ||
| stringstream output; | ||
| output << "\nLAYOUT PARAMETERS" << endl; | ||
| output << "\tEndogenously active neuron positions: "; | ||
|
|
||
| for (boost::tie(vi, vi_end) = gm.vertices(); vi != vi_end; ++vi) { | ||
| assert(*vi < numVertices_); | ||
| if (gm[*vi].active) { | ||
| output << *vi << " "; | ||
| } | ||
| } | ||
| output << endl; | ||
|
|
||
| output << "\tInhibitory neuron positions: "; | ||
|
|
||
| for (boost::tie(vi, vi_end) = gm.vertices(); vi != vi_end; ++vi) { | ||
| assert(*vi < numVertices_); | ||
| if (gm[*vi].type == "INH") { | ||
| output << *vi << " "; | ||
| } | ||
| } | ||
| output << endl; | ||
|
|
||
| LOG4CPLUS_DEBUG(fileLogger_, output.str()); | ||
| } |
There was a problem hiding this comment.
Was this replaced by simpler code in LayourNeuro.cpp?
stiber
left a comment
There was a problem hiding this comment.
I have one question, but this is mergeable unless that question raises a problem.
…eters by model
Closes #843
Description
Moved per-vertex types out of Layout; register history in AllVertices subclasses.
LayoutNeuro owns neuro lists and logging; Layout911 owns role index lists.
Update call sites (connections, recorders, GPU ConnGrowth).
Checklist (Mandatory for new features)
Testing (Mandatory for all changes)
test-medium-connected.xmlPassedtest-large-long.xmlPassed